chore: Add persistence store support for FDv2 #357
Conversation
Bug: Deadlock Risk in FeatureStoreClientWrapperIn Additional Locations (1) |
| self.__lock.unlock() | ||
|
|
||
| if modified: | ||
| self.__listeners.notify(status) |
There was a problem hiding this comment.
What is the threading situation like for this dispatch? Is the caller thread always the same thread?
There was a problem hiding this comment.
Yes, unfortunately that's the way the current listeners work in python. The notify method itself catches all exceptions, which is why we aren't doing it here.
There was a problem hiding this comment.
Ok. Well it is good in a sense, because it helps prevent out of order handling from unfortunate thread context switching. If it was multi-threaded, then I would be concerned we would need some async queue type situation.
| """ | ||
| update_status is called from the data store to push a status update. | ||
| """ | ||
| self.__lock.lock() |
There was a problem hiding this comment.
Is there a reason this has to use lock/unlock vs with?
There was a problem hiding this comment.
This is the LD provided RW lock. There is no context usage for this class.
Note
Adds persistent data store (read-only/read-write) support to FDv2 with status monitoring, configuration hooks, and comprehensive tests.
FeatureStorewith read-only/read-write modes viaFeatureStoreClientWrapper; wiresDataStoreStatusProviderand availability monitoring; exposesdata_store_status_provider.log).DataSystemConfigwithdata_store_modeanddata_store;primary_synchronizernow optional.ConfigBuilderaddsdata_store(...), relaxes build rule (secondary requires primary), and providesdaemon(...)andpersistent_store(...)helpers.DataStoreModeenum; implementsDataStoreStatusProviderImplandDataStoreStatus.__eq__.DataStoreStatusProvider.is_monitoring_enabled()support and propagation from stores.test_fdv2_persistence.py) and updates config builder tests.Written by Cursor Bugbot for commit 4e44909. This will update automatically on new commits. Configure here.